add script output to GitOps#44728
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds GitOps output for scripts and a changelog entry documenting it. Server client logging in ApplyGroup now reports script counts differently for dry-run (uses payload count) and non-dry-run (uses server-returned count) for both no-team and per-team script applications. Adds TestGitOpsScriptsLogging to assert the presence of per-script log lines in dry-run and real-run scenarios for global and team GitOps YAMLs. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@changes/44082-add-script-output-to-gitops`:
- Line 1: Replace the ungrammatical phrase "indicating how many would or be
applied when running." with the corrected wording "indicating how many scripts
would be applied (dry run) or were applied." Locate the string in the change
titled 44082-add-script-output-to-gitops (the commit/message or constant
containing that sentence) and update it to the suggested phrasing so the release
note reads clearly.
In `@cmd/fleetctl/fleetctl/gitops_test.go`:
- Around line 6637-6669: The test writes YAML using env substitutions but never
sets FLEET_SERVER_URL, ORG_NAME, or TEST_TEAM_NAME; set these explicitly before
creating globalPath and teamPath (e.g., call os.Setenv("FLEET_SERVER_URL",
"http://localhost:8080"), os.Setenv("ORG_NAME", "test-org"),
os.Setenv("TEST_TEAM_NAME", "test-team")) and add defers to restore/unset them
(defer os.Unsetenv(...)) so the test is deterministic; update the setup near
tmpDir/globalPath and tmpDir/teamPath creation in gitops_test.go to include
these os.Setenv and cleanup calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec2cb67c-e77c-41e5-8e70-ea7afcaa801f
📒 Files selected for processing (3)
changes/44082-add-script-output-to-gitopscmd/fleetctl/fleetctl/gitops_test.goserver/service/client.go
There was a problem hiding this comment.
Pull request overview
This PR adds GitOps CLI output for script changes so users can see when scripts are applied (or would be applied in --dry-run), addressing the gap described in #44082.
Changes:
- Add logging in
ApplyGroupfor both no-team and team script batch operations (dry-run and real mode). - Add a fleetctl GitOps test asserting the new script-related log lines are present.
- Add a changelog entry noting the new GitOps script output.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/service/client.go | Emit GitOps log lines for script application in dry-run and real runs (no-team + per-team). |
| cmd/fleetctl/fleetctl/gitops_test.go | Add regression test verifying script logging appears in both dry-run and real runs. |
| changes/44082-add-script-output-to-gitops | Document the user-visible GitOps output change for scripts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #44728 +/- ##
==========================================
+ Coverage 66.66% 66.71% +0.05%
==========================================
Files 2672 2674 +2
Lines 214802 215468 +666
Branches 9945 9945
==========================================
+ Hits 143206 143760 +554
- Misses 58564 58641 +77
- Partials 13032 13067 +35
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Unrelated test failures |
MagnusHJensen
force-pushed
the
44082-add-script-output-to-gitops
branch
from
6567bbc to
ab62ecd
Compare
|
@JordanMontgomery I had to rebase to fix merge conflicts |
Related issue: Resolves #44082
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Timeouts are implemented and retries are limited to avoid infinite loops
If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes
Testing
Summary by CodeRabbit
New Features
Tests